Add PaginatedKVStore traits upstreamed from ldk-server #4347

benthecarman · 2026-01-26T15:00:16Z

Allows for a paginated KV store for more efficient listing of keys so you don't need to fetch all at once. Uses monotonic counter or timestamp to track the order of keys and allow for pagination. The traits are largely just copy-pasted from ldk-server.

This also adds a PaginatedKVStoreSyncAdapter/PaginatedKVStoreAdapter so you can use a paginated kv store in contexts that expect a regular key value store.

Adds some basic tests that were generated using claude code.

ldk-reviews-bot · 2026-01-26T15:00:20Z

👋 I see @tnull was un-assigned.
If you'd like another reviewer assignment, please click here.

codecov · 2026-01-26T16:41:29Z

Codecov Report

❌ Patch coverage is 65.47619% with 87 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.04%. Comparing base (3fee76b) to head (ac4e907).
⚠️ Report is 12 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning-persister/src/fs_store.rs	65.71%	28 Missing and 8 partials ⚠️
lightning/src/util/persist.rs	56.00%	33 Missing ⚠️
lightning/src/util/test_utils.rs	75.00%	17 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4347      +/-   ##
==========================================
- Coverage   86.08%   86.04%   -0.05%     
==========================================
  Files         156      156              
  Lines      102416   102714     +298     
  Branches   102416   102714     +298     
==========================================
+ Hits        88165    88377     +212     
- Misses      11759    11835      +76     
- Partials     2492     2502      +10

Flag	Coverage Δ
tests	`86.04% <65.47%> (-0.05%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

TheBlueMatt · 2026-01-26T16:45:11Z

lightning/src/util/persist.rs

+
+	/// A token that can be used to retrieve the next set of keys.
+	/// The `String` is the last key from the current page and the `i64` is
+	/// the associated `time` used for ordering.


I don't think most DBs are going to support this? "sort by the last-changed as of time X" isn't possible without storing what the last-updated time was prior to X when something is updated after X.

ldk-server does it based on creation_at not updated_at, will update the docs to make that more clear

Hmm, do all of our backend storage things have a created-at time? I guess if you want it time-ordered you have to do it that way but it adds quite a requirement :/

for ldk-server's we do

https://github.com/lightningdevkit/ldk-server/blob/9ae5ff8e2d2faf19540ce1be7af973ea134f9465/ldk-server/src/io/persist/sqlite_store/mod.rs#L97

Hmm, presumably that needs to be upstreamed to ldk-node and maybe also vss-server? I imagine we want pagination there too?

lightning/src/util/persist.rs

TheBlueMatt · 2026-01-26T18:09:37Z

lightning/src/util/persist.rs

+/// when dealing with a large number of keys that cannot be efficiently retrieved all at once.
+///
+/// For an asynchronous version of this trait, see [`PaginatedKVStore`].
+pub trait PaginatedKVStoreSync: Send + Sync {


Why doesn't this just extend KVStoreSync? Seems weird to duplicate the interface.

I am too used to having to create wrappers for ldk traits, I am actually in ldk now though! Fixed

You can add extension traits in downstream crates too?

You can add extension traits in downstream crates too?

To give some 'historic' context here, there were discussions on whether or not to make this an extension trait (cf. lightningdevkit/ldk-server#22). In the end, the PR author decided against it, also because it was anticipated that pagination might require two separate implementations.

However, IMO, now would be a good time to at least consider rethinking and double-checking some the design choices made, as there is still time to fundamentally LDK Server's persistence model, before we cut the first release (cc @jkczyz as he was also part of the original discussions, not sure if he has an opinion now).

In particular, I want to throw out some questions to consider:

Do we really want to maintain separate implementations for all backends and have LDK Node take different store types for 'general purpose' vs 'payment data'?

Either way, couldn't we still make PaginatedKVStore an extension trait?

If we will require it in LDK Node for payment data, will we be able to add pagination to all supported backends? if so, should this even be just a breaking change to KVStore::list rather than making it an extension trait?

Somewhat relatedly, note that from my point of view our endgoal should be that also all backend implementations (incl. VSS, SQLite, Postgres) eventually live in lightning-persister, and that preferably very little or no custom logic would live on the LDK Node/Server side. IMO this top-down approach would give clear interfaces and implementations across the board and avoid risking running into edge cases when making conflicting decisions on implementation details in different crates.

Do we really want to maintain separate implementations for all backends and have LDK Node take different store types for 'general purpose' vs 'payment data'?

Ideally not indeed.

Either way, couldn't we still make PaginatedKVStore an extension trait?

Definitely should be (it is now in this PR).

If we will require it in LDK Node for payment data, will we be able to add pagination to all supported backends?

Yea, see above discussion. Requiring that the paginated list be in created_at order means that most of our backends will have to actually store a created_at time (or have an incrementing row id at least). At least the only one in lightning-persister (filesystem store) already has it, but the SQLite/Postgres/VSS backends will all need it.

For that reason I'm really not a fan of sorting by created_at but it does certainly make the client easier. Another option would be to only require that things be listed in key-order and then make sure that when we want pagination we store a key that is ordered, but that doesn't work for VSS cause the key is encrypted garbage (so we'd have to have some crazy abstraction-break there). Dunno if anyone has any more creative ideas.

Somewhat relatedly, note that from my point of view our endgoal should be that also all backend implementations (incl. VSS, SQLite, Postgres) eventually live in lightning-persister, and that preferably very little or no custom logic would live on the LDK Node/Server side. IMO this top-down approach would give clear interfaces and implementations across the board and avoid risking running into edge cases when making conflicting decisions on implementation details in different crates.

👍

benthecarman · 2026-01-28T15:58:55Z

Updated with @TheBlueMatt's suggestions, now only uses last_key as the page token and leaves the time/ordering up to the implementation

Allows for a paginated KV store for more efficient listing of keys so you don't need to fetch all at once. Uses monotonic counter or timestamp to track the order of keys and allow for pagination. The traits are largely just copy-pasted from ldk-server. Adds some basic tests that were generated using claude code.

Test created with claude code

benthecarman · 2026-01-28T19:07:34Z

implemented the paginated kv store for the file system store

benthecarman requested a review from tnull January 26, 2026 15:00

TheBlueMatt reviewed Jan 26, 2026

View reviewed changes

benthecarman force-pushed the paginated-kv-store branch from 5160a11 to 3fb869f Compare January 26, 2026 17:56

TheBlueMatt reviewed Jan 26, 2026

View reviewed changes

benthecarman force-pushed the paginated-kv-store branch 2 times, most recently from ebb2e5d to 95b82f8 Compare January 26, 2026 20:45

tnull removed their request for review January 27, 2026 08:30

benthecarman force-pushed the paginated-kv-store branch 2 times, most recently from ffdf5c9 to 77d2cac Compare January 28, 2026 15:57

benthecarman force-pushed the paginated-kv-store branch from 77d2cac to 8b8b789 Compare January 28, 2026 16:03

Implement PaginatedKVStore for FilesystemStore

ac4e907

Test created with claude code

benthecarman force-pushed the paginated-kv-store branch from 0105148 to ac4e907 Compare January 28, 2026 19:07

Add PaginatedKVStore traits upstreamed from ldk-server #4347

Are you sure you want to change the base?

Add PaginatedKVStore traits upstreamed from ldk-server #4347

Conversation

benthecarman commented Jan 26, 2026

Uh oh!

ldk-reviews-bot commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tnull Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benthecarman commented Jan 28, 2026

Uh oh!

benthecarman commented Jan 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ldk-reviews-bot commented Jan 26, 2026 •

edited

Loading

codecov bot commented Jan 26, 2026 •

edited

Loading

tnull Jan 27, 2026 •

edited

Loading